Split detecting unconstructible pub structs into a new lint from dead_code #128389

mu001999 · 2024-07-30T12:23:12Z

#125572 detects never constructed pub structs, but introduces some regressions (#126169 and #128272).

This does not fix the regressions but just split the detection into a new lint, #127104 and #128329 try to fix them.

I don't know if it is late or not for this PR. But this could at least provide a way to help silence the lint separately for current or potential future regressions.

rustbot · 2024-07-30T12:23:21Z

r? @nnethercote

rustbot has assigned @nnethercote.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

compiler/rustc_lint_defs/src/builtin.rs

mu001999 · 2024-07-30T14:55:18Z

wired failure:

   3 warnings found (use docker --debug to expand):
   - LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format (line 81)
   - LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format (line 86)
   - LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format (line 101)
  Dockerfile:76
  --------------------
    74 |     
    75 |     COPY scripts/nodejs.sh /scripts/
    76 | >>> RUN sh /scripts/nodejs.sh /node
    77 |     ENV PATH="/node/bin:${PATH}"
    78 |     
  --------------------
  ERROR: failed to solve: process "/bin/sh -c sh /scripts/nodejs.sh /node" did not complete successfully: exit code: 2
  The command has failed after 5 attempts.
  Error: Process completed with exit code 1.

🤔is there any other way to manually restart gh jobs other than push new?

compiler-errors · 2024-07-30T15:40:12Z

compiler/rustc_passes/src/dead.rs

+        let lint = if tcx.visibility(first_item.def_id).is_public()
+            && matches!(tcx.def_kind(first_item.def_id), DefKind::Struct)
+        {
+            UNCONSTRUCTIBLE_PUB_STRUCT
+        } else {
+            DEAD_CODE
+        };


It seems very sketchy that this depends on first_item. Doesn't this suppress dead_code if a pub ADT comes first in the list?

It seems very sketchy that this depends on first_item. Doesn't this suppress dead_code if a pub ADT comes first in the list?

No, we will only have one pub struct there. Because this function will only be called by warn_dead_code for struct.

nnethercote · 2024-07-30T21:58:36Z

I will assign to @compiler-errors because he has already started reviewing.

r? @compiler-errors

compiler-errors · 2024-07-31T00:10:33Z

This PR has the side-effect of now suppressing dead_code warnings if we have unreachable-pub structs, such as mod x { pub Unreachable(i32); }. I think the new lint should only trigger for the new dead_code behavior introduced recently.

mu001999 · 2024-07-31T00:31:55Z

@compiler-errors updated and use tcx.effective_visibilities(()).is_reachable now, and add a test for mod x { pub Unreachable(i32); } in tests/ui/lint/dead-code/unconstructible-pub-struct.rs

bors · 2024-08-03T16:19:05Z

☔ The latest upstream changes (presumably #128404) made this pull request unmergeable. Please resolve the merge conflicts.

compiler-errors · 2024-08-05T17:10:24Z

Let's reintroduce this lint later. I will leave it up to you, but this is probably best closed.

@rustbot author

rustbot assigned nnethercote Jul 30, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 30, 2024